*: harden generated Bazel artifact workflow - #68243

.github/workflows/update-bazel-files.yml (2)

195-203: 💤 Low value

Minor: commit-time pathspec is broader than the validated allow-list.

The commit gate uses DEPS.bzl ':(glob)**/*.bazel' ':(glob)**/*.bzl', which is wider than the validation allow-list (which additionally permits only specific build/**/BUILD.bazel shapes). In practice this is fine because the validate step already rejected anything outside the allow-list before we reach this point, so nothing unsafe can be staged here. No change required — noting only because, if the validate step is ever relaxed independently, this gate will silently follow along. A brief comment tying these two patterns together (or factoring the allow-list into a single source of truth) would help future maintainers keep them in sync.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/update-bazel-files.yml around lines 195 - 203, The commit
gate's pathspec (DEPS.bzl ':(glob)**/*.bazel' ':(glob)**/*.bzl') is broader than
the validation allow-list (which restricts to specific build/**/BUILD.bazel
shapes); add a brief comment next to the git commit/push block (around the git
-C pr commit / git -C pr push lines) explaining that the validate step enforces
a stricter allow-list and that these patterns must be kept in sync, or refactor
both the validate step and this pathspec to reference a single source-of-truth
allow-list constant to avoid accidental drift.

---

113-130: 💤 Low value

Consider adding a brief inline comment explaining the case-glob semantics to clarify for future maintainers why build/*/BUILD.bazel (allow) must precede build/* (deny) and how WORKSPACE.bazel is properly caught by the explicit deny before *.bazel could match it. This low-effort addition improves maintainability of the security-sensitive path validation logic.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/update-bazel-files.yml around lines 113 - 130, Add a short
inline comment in the case statement describing the glob-ordering semantics and
why specific patterns must come first: explain that the allow pattern
build/*/BUILD.bazel must precede the deny build/* to avoid being shadowed, and
note that explicit denies like WORKSPACE and WORKSPACE.bazel are listed before
the broader *.bazel rule so workspace files are safely caught; place this
comment near the case(...) block around the build/*/BUILD.bazel, build/*,
WORKSPACE.bazel, and *.bazel patterns.

.github/workflows/check-bazel-prepare.yml (3)

34-40: ⚡ Quick win

Consider enabling caches for improved CI performance.

All caches are currently disabled for both Go (cache: false) and Bazel (all three cache types). While this ensures reproducibility and avoids cache-related issues, it can significantly slow down the workflow, especially for a check that runs on every PR update.

Unless there's a specific reason for disabling caches (e.g., reproducibility requirements, cache corruption issues, or disk space constraints), consider enabling them to improve CI performance and developer experience.

💡 Proposed change to enable caches

       - name: Set up Go
         uses: actions/setup-go@v6
         with:
           go-version: 1.25
-          cache: false
+          cache: true
       - name: Set up Bazelisk
         uses: bazel-contrib/setup-bazel@0.16.0
         with:
-          bazelisk-cache: false
-          repository-cache: false
-          external-cache: false
+          bazelisk-cache: true
+          repository-cache: true
+          external-cache: true

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/check-bazel-prepare.yml around lines 34 - 40, The workflow
currently disables all caches (the Go step uses "cache: false" and the "Set up
Bazelisk" step sets bazelisk-cache, repository-cache, and external-cache to
false), which slows CI; update the workflow to enable caching by changing
"cache: false" to true for the Go/cache step and set bazelisk-cache,
repository-cache, and external-cache to true (or remove those false overrides)
in the "Set up Bazelisk" step (bazel-contrib/setup-bazel@0.16.0) so Bazel and Go
use the available caches unless there is a documented reason to keep them
disabled.

---

28-28: ⚡ Quick win

Optimize checkout performance by using shallow clone.

The check-bazel-prepare.sh script only uses git diff to compare working directory changes, which does not require full git history. Change fetch-depth: 0 to fetch-depth: 1 to speed up the workflow without affecting functionality.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/check-bazel-prepare.yml at line 28, The checkout step
currently sets fetch-depth: 0 which fetches full history; change the GitHub
Actions checkout configuration from the literal "fetch-depth: 0" to
"fetch-depth: 1" so the workflow performs a shallow clone (only the latest
commit) — locate the checkout step that contains the "fetch-depth" key in
.github/workflows/check-bazel-prepare.yml and update its value to 1.

---

30-34: Consider updating to Go 1.26.3, the latest stable version (released May 7, 2026).

Go 1.25 is valid, but unless intentionally specified for compatibility reasons, updating to Go 1.26.3 is recommended.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/check-bazel-prepare.yml around lines 30 - 34, Update the
GitHub Actions step that uses actions/setup-go@v6: change the go-version value
from 1.25 to 1.26.3 (the current stable release) so the workflow runs with Go
1.26.3; keep other step keys (uses, cache) unchanged unless there is an
intentional compatibility constraint requiring 1.25.

.github/workflows/update-bazel-files.yml (1)

96-100: ⚡ Quick win

Distinguish missing patch file from empty patch.

The check [[ ! -s "${patch_path}" ]] returns true for both non-existent and empty files. If the artifact is malformed and bazel.patch is missing entirely, the job silently succeeds. Consider adding an explicit existence check first:

+if [[ ! -f "${patch_path}" ]]; then
+  echo "Error: bazel.patch not found in artifact for fork PR #${PR_NUMBER}." >&2
+  exit 1
+fi
 if [[ ! -s "${patch_path}" ]]; then
   echo "No generated Bazel changes for fork PR #${PR_NUMBER}."
   exit 0
 fi

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/update-bazel-files.yml around lines 96 - 100, The current
check using patch_path and [[ ! -s "${patch_path}" ]] conflates missing and
empty bazel.patch; change the logic to first test existence with [[ ! -f
"${patch_path}" ]] (or [[ ! -e "${patch_path}" ]]) and, if missing, emit an
error message (e.g., "Missing bazel.patch artifact") and exit non-zero so the
job fails; only after confirming the file exists, use [[ ! -s "${patch_path}" ]]
to detect an empty patch and handle that case (keep the existing "No generated
Bazel changes..." message and exit 0).

.github/workflows/check-bazel-prepare.yml (1)

40-40: 💤 Low value

Document why CI is unset.

Unsetting the CI environment variable before running make check-bazel-prepare suggests the make target has different behavior in CI vs local contexts. Add a comment explaining why this is necessary to avoid confusion for future maintainers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/check-bazel-prepare.yml at line 40, Add a brief inline
comment immediately above the unset CI line explaining why CI is being cleared
before running make check-bazel-prepare: state that the make target detects CI
and alters behavior (e.g., skips local-only setup, changes caching or
credentials), and that unsetting CI forces the local execution path needed for
this workflow; reference the make target name make check-bazel-prepare and the
unset CI command so future maintainers understand the intentional override.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes